Reconstruct the risk analysis treatment plans as of a date - #1782
Conversation
f99eb1f to
dfef3e7
Compare
dfef3e7 to
0b35996
Compare
|
@cubic-dev-ai please review |
@SachaProbo I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 42 files
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
4803510 to
6e0415d
Compare
6e0415d to
8e143fe
Compare
There was a problem hiding this comment.
All reported issues were addressed across 48 files (changes from recent commits).
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
e047f4c to
f0539a7
Compare
f0539a7 to
9fa10b0
Compare
There was a problem hiding this comment.
All reported issues were addressed across 48 files (changes from recent commits).
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
9fa10b0 to
ce303d1
Compare
There was a problem hiding this comment.
All reported issues were addressed across 49 files (changes from recent commits).
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
ce303d1 to
0dc2e51
Compare
There was a problem hiding this comment.
3 issues found across 49 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/coredata/measure_event.go">
<violation number="1" location="pkg/coredata/measure_event.go:66">
P2: Historical measure nodes always return a nil description. `MeasureEvent` does not snapshot `Description`, so `Measure()` drops any description that existed at the requested instant. Persist and restore the description in measure events, including migration backfill and event writers.</violation>
</file>
<file name="apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_components/TreatmentPlanProgressBar.tsx">
<violation number="1" location="apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_components/TreatmentPlanProgressBar.tsx:50">
P2: Screen-reader users cannot determine the in-progress or not-implemented counts because this bar exposes status only through color and the visible text reports `done/total`. Add an accessible progress label or visually hidden legend containing all three status counts, with localized status names.</violation>
</file>
<file name="pkg/probo/measure_service.go">
<violation number="1" location="pkg/probo/measure_service.go:476">
P1: If a delete commits after this update loads the measure, the zero-row update still records an UPDATE event, making the deleted measure reappear in as-of reads. Serialize the operations or verify the update affected a row before recording the event.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| eventType = coredata.MeasureEventTypeUpdated | ||
| } | ||
|
|
||
| if err := insertMeasureEvent( |
There was a problem hiding this comment.
P1: If a delete commits after this update loads the measure, the zero-row update still records an UPDATE event, making the deleted measure reappear in as-of reads. Serialize the operations or verify the update affected a row before recording the event.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/probo/measure_service.go, line 476:
<comment>If a delete commits after this update loads the measure, the zero-row update still records an UPDATE event, making the deleted measure reappear in as-of reads. Serialize the operations or verify the update affected a row before recording the event.</comment>
<file context>
@@ -463,10 +463,27 @@ func (s MeasureService) Import(
+ eventType = coredata.MeasureEventTypeUpdated
+ }
+
+ if err := insertMeasureEvent(
+ ctx,
+ tx,
</file context>
0dc2e51 to
818b61f
Compare
There was a problem hiding this comment.
All reported issues were addressed across 50 files (changes from recent commits).
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
818b61f to
84b170e
Compare
There was a problem hiding this comment.
All reported issues were addressed across 50 files (changes from recent commits).
Requires human review: Auto-approval blocked because this review re-detected 4 unresolved issues already reported by Cubic.
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
9fbf4d4 to
962eb39
Compare
There was a problem hiding this comment.
1 existing issue remains and 11 new issues found across 51 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_components/TreatmentPlanListItem.tsx">
<violation number="1" location="apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_components/TreatmentPlanListItem.tsx:70">
P1: When a user has treatment-plan list permission without treatment-plan get permission, this fragment now requests `progress` and makes the list fail. Use a list-authorized progress path or align the resolver authorization with the surrounding list query.</violation>
</file>
<file name="pkg/coredata/measure_filter.go">
<violation number="1" location="pkg/coredata/measure_filter.go:85">
P3: `EventSQLFragment` duplicates nearly all of `SQLFragment`, so future filter changes can silently diverge between live and as-of reads. Extract the shared predicates into a helper and parameterize only the storage-specific expressions.</violation>
<violation number="2" location="pkg/coredata/measure_filter.go:93">
P2: When an as-of search contains tsquery punctuation such as `foo & bar`, this construction produces an invalid query and the request fails instead of returning results. Escape each token or use a plain/web-search parser, and apply the same correction to `SQLFragment`.</violation>
</file>
<file name="pkg/coredata/migrations/20260826T135100Z.sql">
<violation number="1" location="pkg/coredata/migrations/20260826T135100Z.sql:117">
P1: When an existing plan or measure was edited before this migration, the seed records its current snapshot at the original creation time, fabricating history for later as-of dates. Stamp seed events at migration time instead, so pre-migration history remains unavailable as documented.</violation>
</file>
<file name="pkg/coredata/treatment_plan_event.go">
<violation number="1" location="pkg/coredata/treatment_plan_event.go:259">
P2: When two events for one treatment plan share a timestamp, PostgreSQL may choose either row for `DISTINCT ON`. As-of reads can therefore return stale treatment or measure data; add a monotonic event sequence/ID and order by it in both selectors.</violation>
</file>
<file name="pkg/coredata/treatment_plan.go">
<violation number="1" location="pkg/coredata/treatment_plan.go:699">
P3: The new historical treatment-plan queries cross the entity boundary to read `measure_events` for net-score filtering. Move historical measure-state loading to the measure layer or caller, then pass the resulting IDs/states into treatment-plan queries instead of joining measure events here.
(Based on your team's feedback about cross-entity SQL joins.)</violation>
</file>
<file name="pkg/probo/measure_service.go">
<violation number="1" location="pkg/probo/measure_service.go:617">
P2: When concurrent updates commit out of start order, the event timestamp can be older than the update that committed first, so as-of reads select the wrong final measure state. Lock the measure before timestamping and updating, or generate the event timestamp after acquiring the database row lock.</violation>
</file>
<file name="apps/console/src/pages/organizations/risks/risk-analyses/_components/RiskAnalysisPlansSection.tsx">
<violation number="1" location="apps/console/src/pages/organizations/risks/risk-analyses/_components/RiskAnalysisPlansSection.tsx:405">
P2: When switching from a historical date back to today, stale historical rows remain editable while the live-data refetch is pending. Keep plans read-only until the new live data is ready.</violation>
</file>
<file name="pkg/cmd/risk-analysis/view/view.go">
<violation number="1" location="pkg/cmd/risk-analysis/view/view.go:124">
P2: `time.Parse(time.RFC3339, ...)` normalizes out-of-range values instead of rejecting them, so the guard doesn't guarantee a valid timestamp. For example "2026-02-30T00:00:00Z" returns nil error and is sent as-is, then the server normalizes it to March 2, silently reconstructing cells as of an unintended date. Validate the semantic date (e.g. confirm the parsed value round-trips, or compare parsed month/day to the input) and send the canonical parsed value rather than the raw user string.</violation>
</file>
<file name="apps/console/src/pages/organizations/risks/risk-analyses/_lib/useMatrixAsOf.ts">
<violation number="1" location="apps/console/src/pages/organizations/risks/risk-analyses/_lib/useMatrixAsOf.ts:52">
P2: When this page remains open across local midnight without an `asOf` parameter, `asOfDate` stays yesterday because the memo only reruns when `search` changes; the page then switches from live data to historical data. Track the current date and refresh the derived value at the day boundary so an unpinned view remains live.</violation>
</file>
<file name="apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_components/TreatmentPlanProgressBar.tsx">
<violation number="1" location="apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_components/TreatmentPlanProgressBar.tsx:41">
P2: When a treatment plan contains a `NOT_STARTED`, `NOT_APPLICABLE`, or `UNKNOWN` measure, `total` includes it but `paintedEnd` does not, leaving that portion of the progress bar as an unexplained background segment while the label still reports it in the denominator. Count every valid state in a displayed bucket or add an explicit unclassified segment.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| canDelete: permission(action: "risk-management:treatment-plan:delete") | ||
| measureCount: measures(first: 0) { | ||
| totalCount | ||
| progress { |
There was a problem hiding this comment.
P1: When a user has treatment-plan list permission without treatment-plan get permission, this fragment now requests progress and makes the list fail. Use a list-authorized progress path or align the resolver authorization with the surrounding list query.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_components/TreatmentPlanListItem.tsx, line 70:
<comment>When a user has treatment-plan list permission without treatment-plan get permission, this fragment now requests `progress` and makes the list fail. Use a list-authorized progress path or align the resolver authorization with the surrounding list query.</comment>
<file context>
@@ -41,46 +41,40 @@ import { updateStoreCounter } from "#/hooks/useMutationWithIncrement";
canDelete: permission(action: "risk-management:treatment-plan:delete")
- measureCount: measures(first: 0) {
- totalCount
+ progress {
+ done
+ inProgress
</file context>
| ARRAY[]::TEXT[] | ||
| ), | ||
| r.category, | ||
| tp.created_at, |
There was a problem hiding this comment.
P1: When an existing plan or measure was edited before this migration, the seed records its current snapshot at the original creation time, fabricating history for later as-of dates. Stamp seed events at migration time instead, so pre-migration history remains unavailable as documented.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/migrations/20260826T135100Z.sql, line 117:
<comment>When an existing plan or measure was edited before this migration, the seed records its current snapshot at the original creation time, fabricating history for later as-of dates. Stamp seed events at migration time instead, so pre-migration history remains unavailable as documented.</comment>
<file context>
@@ -0,0 +1,144 @@
+ ARRAY[]::TEXT[]
+ ),
+ r.category,
+ tp.created_at,
+ tp.updated_at,
+ tp.created_at
</file context>
| export function useMatrixAsOf() { | ||
| const [params, setParams] = useSearchParams(); | ||
| const search = params.toString(); | ||
| const asOfDate = useMemo( |
There was a problem hiding this comment.
P2: When this page remains open across local midnight without an asOf parameter, asOfDate stays yesterday because the memo only reruns when search changes; the page then switches from live data to historical data. Track the current date and refresh the derived value at the day boundary so an unpinned view remains live.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/risks/risk-analyses/_lib/useMatrixAsOf.ts, line 52:
<comment>When this page remains open across local midnight without an `asOf` parameter, `asOfDate` stays yesterday because the memo only reruns when `search` changes; the page then switches from live data to historical data. Track the current date and refresh the derived value at the day boundary so an unpinned view remains live.</comment>
<file context>
@@ -0,0 +1,71 @@
+export function useMatrixAsOf() {
+ const [params, setParams] = useSearchParams();
+ const search = params.toString();
+ const asOfDate = useMemo(
+ () => parseAsOfDate(new URLSearchParams(search)),
+ [search],
</file context>
| TRUE | ||
| ELSE | ||
| to_tsvector('simple', name) @@ ( | ||
| SELECT to_tsquery('simple', string_agg(lexeme || ':*', ' & ')) |
There was a problem hiding this comment.
P2: When an as-of search contains tsquery punctuation such as foo & bar, this construction produces an invalid query and the request fails instead of returning results. Escape each token or use a plain/web-search parser, and apply the same correction to SQLFragment.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/measure_filter.go, line 93:
<comment>When an as-of search contains tsquery punctuation such as `foo & bar`, this construction produces an invalid query and the request fails instead of returning results. Escape each token or use a plain/web-search parser, and apply the same correction to `SQLFragment`.</comment>
<file context>
@@ -81,3 +81,37 @@ AND
+ TRUE
+ ELSE
+ to_tsvector('simple', name) @@ (
+ SELECT to_tsquery('simple', string_agg(lexeme || ':*', ' & '))
+ FROM unnest(regexp_split_to_array(trim(@query), '\s+')) AS lexeme
+ )
</file context>
| treatmentPlanKey={row.plan} | ||
| connectionId={connectionId} | ||
| matrixSize={matrixSize} | ||
| readOnly={hasAsOf} |
There was a problem hiding this comment.
P2: When switching from a historical date back to today, stale historical rows remain editable while the live-data refetch is pending. Keep plans read-only until the new live data is ready.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/risks/risk-analyses/_components/RiskAnalysisPlansSection.tsx, line 405:
<comment>When switching from a historical date back to today, stale historical rows remain editable while the live-data refetch is pending. Keep plans read-only until the new live data is ready.</comment>
<file context>
@@ -239,109 +333,174 @@ function RiskAnalysisPlansTable({
+ treatmentPlanKey={row.plan}
+ connectionId={connectionId}
+ matrixSize={matrixSize}
+ readOnly={hasAsOf}
+ onChanged={reload}
+ />
</file context>
| readOnly={hasAsOf} | |
| readOnly={hasAsOf || !unplannedReady || isPending} |
| const inProgressEnd = total === 0 | ||
| ? 0 | ||
| : Math.round(((done + inProgress) / total) * 100); | ||
| const paintedEnd = total === 0 |
There was a problem hiding this comment.
P2: When a treatment plan contains a NOT_STARTED, NOT_APPLICABLE, or UNKNOWN measure, total includes it but paintedEnd does not, leaving that portion of the progress bar as an unexplained background segment while the label still reports it in the denominator. Count every valid state in a displayed bucket or add an explicit unclassified segment.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_components/TreatmentPlanProgressBar.tsx, line 41:
<comment>When a treatment plan contains a `NOT_STARTED`, `NOT_APPLICABLE`, or `UNKNOWN` measure, `total` includes it but `paintedEnd` does not, leaving that portion of the progress bar as an unexplained background segment while the label still reports it in the denominator. Count every valid state in a displayed bucket or add an explicit unclassified segment.</comment>
<file context>
@@ -0,0 +1,74 @@
+ const inProgressEnd = total === 0
+ ? 0
+ : Math.round(((done + inProgress) / total) * 100);
+ const paintedEnd = total === 0
+ ? 0
+ : Math.round(((done + inProgress + notImplemented) / total) * 100);
</file context>
| var asOf *string | ||
|
|
||
| if flagAsOf != "" { | ||
| if _, err := time.Parse(time.RFC3339, flagAsOf); err != nil { |
There was a problem hiding this comment.
P2: time.Parse(time.RFC3339, ...) normalizes out-of-range values instead of rejecting them, so the guard doesn't guarantee a valid timestamp. For example "2026-02-30T00:00:00Z" returns nil error and is sent as-is, then the server normalizes it to March 2, silently reconstructing cells as of an unintended date. Validate the semantic date (e.g. confirm the parsed value round-trips, or compare parsed month/day to the input) and send the canonical parsed value rather than the raw user string.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/cmd/risk-analysis/view/view.go, line 124:
<comment>`time.Parse(time.RFC3339, ...)` normalizes out-of-range values instead of rejecting them, so the guard doesn't guarantee a valid timestamp. For example "2026-02-30T00:00:00Z" returns nil error and is sent as-is, then the server normalizes it to March 2, silently reconstructing cells as of an unintended date. Validate the semantic date (e.g. confirm the parsed value round-trips, or compare parsed month/day to the input) and send the canonical parsed value rather than the raw user string.</comment>
<file context>
@@ -114,9 +118,22 @@ func NewCmdView(f *cmdutil.Factory) *cobra.Command {
+ var asOf *string
+
+ if flagAsOf != "" {
+ if _, err := time.Parse(time.RFC3339, flagAsOf); err != nil {
+ return fmt.Errorf(
+ "--as-of must be RFC3339 (e.g. 2026-01-15T23:59:59Z): %w",
</file context>
| ` | ||
| } | ||
|
|
||
| func (f *MeasureFilter) EventSQLFragment() string { |
There was a problem hiding this comment.
P3: EventSQLFragment duplicates nearly all of SQLFragment, so future filter changes can silently diverge between live and as-of reads. Extract the shared predicates into a helper and parameterize only the storage-specific expressions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/measure_filter.go, line 85:
<comment>`EventSQLFragment` duplicates nearly all of `SQLFragment`, so future filter changes can silently diverge between live and as-of reads. Extract the shared predicates into a helper and parameterize only the storage-specific expressions.</comment>
<file context>
@@ -81,3 +81,37 @@ AND
`
}
+
+func (f *MeasureFilter) EventSQLFragment() string {
+ return `
+(
</file context>
| SELECT latest_event.state | ||
| FROM ( | ||
| SELECT me.state, me.event_type | ||
| FROM measure_events me |
There was a problem hiding this comment.
P3: The new historical treatment-plan queries cross the entity boundary to read measure_events for net-score filtering. Move historical measure-state loading to the measure layer or caller, then pass the resulting IDs/states into treatment-plan queries instead of joining measure events here.
(Based on your team's feedback about cross-entity SQL joins.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/treatment_plan.go, line 699:
<comment>The new historical treatment-plan queries cross the entity boundary to read `measure_events` for net-score filtering. Move historical measure-state loading to the measure layer or caller, then pass the resulting IDs/states into treatment-plan queries instead of joining measure events here.
(Based on your team's feedback about cross-entity SQL joins.) </comment>
<file context>
@@ -517,6 +595,321 @@ WHERE %s
+ SELECT latest_event.state
+ FROM (
+ SELECT me.state, me.event_type
+ FROM measure_events me
+ WHERE me.measure_id = mid
+ AND me.created_at < @as_of
</file context>
962eb39 to
600ca5f
Compare
There was a problem hiding this comment.
3 existing issues remain and 7 new issues found across 51 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/coredata/measure_event.go">
<violation number="1" location="pkg/coredata/measure_event.go:153">
P3: This method duplicates the as-of event-folding query in `Measures.LoadByIDsAsOf`, so fixes to ordering, visibility, or filtering can produce inconsistent historical results. Extract the shared latest-event selection or reuse one implementation instead of maintaining both queries.</violation>
</file>
<file name="apps/console/src/pages/organizations/risks/risk-analyses/_components/RiskAnalysisMatrices.tsx">
<violation number="1" location="apps/console/src/pages/organizations/risks/risk-analyses/_components/RiskAnalysisMatrices.tsx:110">
P2: When a viewer west of UTC opens an analysis created just after midnight UTC, this minimum permits an as-of instant before the analysis existed and shows an empty snapshot. Align `asOfDateBounds` and `matrixAsOf` to the same timezone so the minimum cannot query before creation.</violation>
</file>
<file name="pkg/coredata/treatment_plan_event.go">
<violation number="1" location="pkg/coredata/treatment_plan_event.go:208">
P2: When an analysis contains many treatment plans, an as-of matrix request materializes every latest event and plan in memory through this unbounded loader. Page the reconstruction or aggregate the as-of matrix cells in SQL.</violation>
</file>
<file name="pkg/coredata/measure_event_type.go">
<violation number="1" location="pkg/coredata/measure_event_type.go:38">
P3: This enum omits the standard `MeasureEventTypes()` helper, so callers must duplicate the valid-value list instead of enumerating it from coredata. Add the helper alongside the constants.</violation>
</file>
<file name="pkg/cmd/treatment-plan/view/view.go">
<violation number="1" location="pkg/cmd/treatment-plan/view/view.go:188">
P3: When a treatment plan's own `category` is empty but its linked risk carries a category, `list` shows the risk's category but `view` renders an empty "Category:" line for the same record. Match the list command's behavior: query `risk { category }` and fall back to it when `p.Category` is empty.</violation>
</file>
<file name="pkg/coredata/treatment_plan_event_type.go">
<violation number="1" location="pkg/coredata/treatment_plan_event_type.go:32">
P3: This enum omits the pluralized list helper that every sibling string enum defines (e.g. AiSystemStatuses(), AccessReviewCampaignStatuses()) and that production validation code relies on via validator.OneOfSlice(...). Add a TreatmentPlanEventTypes() []TreatmentPlanEventType returning all five constants so validation/filtering callers can enumerate valid values instead of hardcoding or omitting checks.</violation>
<violation number="2" location="pkg/coredata/treatment_plan_event_type.go:46">
P3: This enum adds IsValid/MarshalText/UnmarshalText logic but no test file, unlike every sibling string enum which ships a _test.go covering each valid value and the invalid-value error path. Add treatment_plan_event_type_test.go matching the pattern (Test...StatusIsValid/UnmarshalText/MarshalText).</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 4 unresolved issues already reported by Cubic.
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
|
||
| const matrixSize = { rows: analysis.matrixSize.rows, cols: analysis.matrixSize.cols }; | ||
| const counts = analysis.matrixCells ?? []; | ||
| const { minDate, maxDate } = asOfDateBounds(analysis.createdAt); |
There was a problem hiding this comment.
P2: When a viewer west of UTC opens an analysis created just after midnight UTC, this minimum permits an as-of instant before the analysis existed and shows an empty snapshot. Align asOfDateBounds and matrixAsOf to the same timezone so the minimum cannot query before creation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/risks/risk-analyses/_components/RiskAnalysisMatrices.tsx, line 110:
<comment>When a viewer west of UTC opens an analysis created just after midnight UTC, this minimum permits an as-of instant before the analysis existed and shows an empty snapshot. Align `asOfDateBounds` and `matrixAsOf` to the same timezone so the minimum cannot query before creation.</comment>
<file context>
@@ -94,7 +107,13 @@ export function RiskAnalysisMatrices({ analysisKey }: RiskAnalysisMatricesProps)
const matrixSize = { rows: analysis.matrixSize.rows, cols: analysis.matrixSize.cols };
- const counts = analysis.matrixCells ?? [];
+ const { minDate, maxDate } = asOfDateBounds(analysis.createdAt);
+ const cells = analysis.matrixCells;
+
</file context>
| return nil | ||
| } | ||
|
|
||
| func (es *TreatmentPlanEvents) LoadLatestByRiskAnalysisIDAsOf( |
There was a problem hiding this comment.
P2: When an analysis contains many treatment plans, an as-of matrix request materializes every latest event and plan in memory through this unbounded loader. Page the reconstruction or aggregate the as-of matrix cells in SQL.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/treatment_plan_event.go, line 208:
<comment>When an analysis contains many treatment plans, an as-of matrix request materializes every latest event and plan in memory through this unbounded loader. Page the reconstruction or aggregate the as-of matrix cells in SQL.</comment>
<file context>
@@ -0,0 +1,379 @@
+ return nil
+}
+
+func (es *TreatmentPlanEvents) LoadLatestByRiskAnalysisIDAsOf(
+ ctx context.Context,
+ conn pg.Querier,
</file context>
| measure_created_at, | ||
| created_at | ||
| FROM ( | ||
| SELECT DISTINCT ON (measure_id) |
There was a problem hiding this comment.
P3: This method duplicates the as-of event-folding query in Measures.LoadByIDsAsOf, so fixes to ordering, visibility, or filtering can produce inconsistent historical results. Extract the shared latest-event selection or reuse one implementation instead of maintaining both queries.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/measure_event.go, line 153:
<comment>This method duplicates the as-of event-folding query in `Measures.LoadByIDsAsOf`, so fixes to ordering, visibility, or filtering can produce inconsistent historical results. Extract the shared latest-event selection or reuse one implementation instead of maintaining both queries.</comment>
<file context>
@@ -0,0 +1,200 @@
+ measure_created_at,
+ created_at
+FROM (
+ SELECT DISTINCT ON (measure_id)
+ organization_id,
+ measure_id,
</file context>
| MeasureEventTypeDeleted MeasureEventType = "DELETED" | ||
| ) | ||
|
|
||
| var ( |
There was a problem hiding this comment.
P3: This enum omits the standard MeasureEventTypes() helper, so callers must duplicate the valid-value list instead of enumerating it from coredata. Add the helper alongside the constants.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/measure_event_type.go, line 38:
<comment>This enum omits the standard `MeasureEventTypes()` helper, so callers must duplicate the valid-value list instead of enumerating it from coredata. Add the helper alongside the constants.</comment>
<file context>
@@ -0,0 +1,73 @@
+ MeasureEventTypeDeleted MeasureEventType = "DELETED"
+)
+
+var (
+ _ fmt.Stringer = MeasureEventType("")
+ _ encoding.TextMarshaler = MeasureEventType("")
</file context>
| _, _ = fmt.Fprintf(out, "%s%s\n", label.Render("Risk:"), p.Risk.ID) | ||
| _, _ = fmt.Fprintf(out, "%s%s\n", label.Render("Risk Analysis:"), p.RiskAnalysis.ID) | ||
| _, _ = fmt.Fprintf(out, "%s%s\n", label.Render("Treatment:"), p.Treatment) | ||
| _, _ = fmt.Fprintf(out, "%s%s\n", label.Render("Category:"), p.Category) |
There was a problem hiding this comment.
P3: When a treatment plan's own category is empty but its linked risk carries a category, list shows the risk's category but view renders an empty "Category:" line for the same record. Match the list command's behavior: query risk { category } and fall back to it when p.Category is empty.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/cmd/treatment-plan/view/view.go, line 188:
<comment>When a treatment plan's own `category` is empty but its linked risk carries a category, `list` shows the risk's category but `view` renders an empty "Category:" line for the same record. Match the list command's behavior: query `risk { category }` and fall back to it when `p.Category` is empty.</comment>
<file context>
@@ -183,6 +185,7 @@ func NewCmdView(f *cmdutil.Factory) *cobra.Command {
_, _ = fmt.Fprintf(out, "%s%s\n", label.Render("Risk:"), p.Risk.ID)
_, _ = fmt.Fprintf(out, "%s%s\n", label.Render("Risk Analysis:"), p.RiskAnalysis.ID)
_, _ = fmt.Fprintf(out, "%s%s\n", label.Render("Treatment:"), p.Treatment)
+ _, _ = fmt.Fprintf(out, "%s%s\n", label.Render("Category:"), p.Category)
_, _ = fmt.Fprintf(out, "%s%s (%s)\n", label.Render("Owner:"), p.Owner.FullName, p.Owner.ID)
</file context>
| TreatmentPlanEventType string | ||
| ) | ||
|
|
||
| const ( |
There was a problem hiding this comment.
P3: This enum omits the pluralized list helper that every sibling string enum defines (e.g. AiSystemStatuses(), AccessReviewCampaignStatuses()) and that production validation code relies on via validator.OneOfSlice(...). Add a TreatmentPlanEventTypes() []TreatmentPlanEventType returning all five constants so validation/filtering callers can enumerate valid values instead of hardcoding or omitting checks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/treatment_plan_event_type.go, line 32:
<comment>This enum omits the pluralized list helper that every sibling string enum defines (e.g. AiSystemStatuses(), AccessReviewCampaignStatuses()) and that production validation code relies on via validator.OneOfSlice(...). Add a TreatmentPlanEventTypes() []TreatmentPlanEventType returning all five constants so validation/filtering callers can enumerate valid values instead of hardcoding or omitting checks.</comment>
<file context>
@@ -0,0 +1,77 @@
+ TreatmentPlanEventType string
+)
+
+const (
+ TreatmentPlanEventTypeCreated TreatmentPlanEventType = "CREATED"
+ TreatmentPlanEventTypeUpdated TreatmentPlanEventType = "UPDATED"
</file context>
| @@ -0,0 +1,77 @@ | |||
| // Copyright (c) 2026 Probo Inc <hello@probo.com>. | |||
There was a problem hiding this comment.
P3: This enum adds IsValid/MarshalText/UnmarshalText logic but no test file, unlike every sibling string enum which ships a _test.go covering each valid value and the invalid-value error path. Add treatment_plan_event_type_test.go matching the pattern (Test...StatusIsValid/UnmarshalText/MarshalText).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/treatment_plan_event_type.go, line 46:
<comment>This enum adds IsValid/MarshalText/UnmarshalText logic but no test file, unlike every sibling string enum which ships a _test.go covering each valid value and the invalid-value error path. Add treatment_plan_event_type_test.go matching the pattern (Test...StatusIsValid/UnmarshalText/MarshalText).</comment>
<file context>
@@ -0,0 +1,77 @@
+ _ encoding.TextUnmarshaler = (*TreatmentPlanEventType)(nil)
+)
+
+func (v TreatmentPlanEventType) IsValid() bool {
+ switch v {
+ case
</file context>
600ca5f to
8766154
Compare
There was a problem hiding this comment.
3 existing issues remain and 5 new issues found across 51 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_components/TreatmentPlanMeasureList.tsx">
<violation number="1" location="apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_components/TreatmentPlanMeasureList.tsx:344">
P3: For an as-of query, `measure.name === ""` can never reach this component because the as-of service filters deleted events before building the response. Remove this dead branch or return deleted-measure tombstones from the service; otherwise deleted measures are not represented by this UI.</violation>
</file>
<file name="pkg/riskmanagement/as_of.go">
<violation number="1" location="pkg/riskmanagement/as_of.go:450">
P2: When a measure is linked to plans from many analyses, this loop issues two database queries per distinct analysis for one page. Batch the as-of event and measure lookups for all plans in the page to avoid query latency growing with the number of analyses.</violation>
</file>
<file name="pkg/coredata/treatment_plan.go">
<violation number="1" location="pkg/coredata/treatment_plan.go:242">
P3: This new treatment-plan loader crosses the coredata entity boundary by joining `risks`. Fetch the risk data separately and keep this treatment-plan query limited to `treatment_plans`.</violation>
<violation number="2" location="pkg/coredata/treatment_plan.go:607">
P3: The four new as-of methods duplicate the same reconstruction and filtering SQL, so future fixes can make counts disagree with page results. Extract the shared as-of CTE and filter expression.</violation>
</file>
<file name="pkg/coredata/measure.go">
<violation number="1" location="pkg/coredata/measure.go:408">
P2: When an as-of measure has a description, `LoadByIDsAsOf` always returns `nil` for it (and an empty `reference_id`), so historical reads do not match the selected measure version. Persist these fields in `measure_events` and select their event values instead of constants.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 4 unresolved issues already reported by Cubic.
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| byAnalysis[plan.RiskAnalysisID] = append(byAnalysis[plan.RiskAnalysisID], plan) | ||
| } | ||
|
|
||
| for analysisID, group := range byAnalysis { |
There was a problem hiding this comment.
P2: When a measure is linked to plans from many analyses, this loop issues two database queries per distinct analysis for one page. Batch the as-of event and measure lookups for all plans in the page to avoid query latency growing with the number of analyses.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/riskmanagement/as_of.go, line 450:
<comment>When a measure is linked to plans from many analyses, this loop issues two database queries per distinct analysis for one page. Batch the as-of event and measure lookups for all plans in the page to avoid query latency growing with the number of analyses.</comment>
<file context>
@@ -0,0 +1,630 @@
+ byAnalysis[plan.RiskAnalysisID] = append(byAnalysis[plan.RiskAnalysisID], plan)
+ }
+
+ for analysisID, group := range byAnalysis {
+ progress, _, err := loadAsOfPlanExtras(
+ ctx,
</file context>
| latest.organization_id, | ||
| latest.category, | ||
| latest.name, | ||
| CAST(NULL AS text) AS description, |
There was a problem hiding this comment.
P2: When an as-of measure has a description, LoadByIDsAsOf always returns nil for it (and an empty reference_id), so historical reads do not match the selected measure version. Persist these fields in measure_events and select their event values instead of constants.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/measure.go, line 408:
<comment>When an as-of measure has a description, `LoadByIDsAsOf` always returns `nil` for it (and an empty `reference_id`), so historical reads do not match the selected measure version. Persist these fields in `measure_events` and select their event values instead of constants.</comment>
<file context>
@@ -326,6 +362,179 @@ WHERE %s
+ latest.organization_id,
+ latest.category,
+ latest.name,
+ CAST(NULL AS text) AS description,
+ latest.state,
+ '' AS reference_id,
</file context>
| const { t } = useTranslation(); | ||
| const organizationId = useOrganizationId(); | ||
| const measure = useFragment(measureFragment, measureKey); | ||
| const deleted = measure.name === ""; |
There was a problem hiding this comment.
P3: For an as-of query, measure.name === "" can never reach this component because the as-of service filters deleted events before building the response. Remove this dead branch or return deleted-measure tombstones from the service; otherwise deleted measures are not represented by this UI.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_components/TreatmentPlanMeasureList.tsx, line 344:
<comment>For an as-of query, `measure.name === ""` can never reach this component because the as-of service filters deleted events before building the response. Remove this dead branch or return deleted-measure tombstones from the service; otherwise deleted measures are not represented by this UI.</comment>
<file context>
@@ -309,26 +328,43 @@ export function TreatmentPlanMeasureList({
const { t } = useTranslation();
const organizationId = useOrganizationId();
const measure = useFragment(measureFragment, measureKey);
+ const deleted = measure.name === "";
return (
</file context>
| filter *TreatmentPlanFilter, | ||
| ) (int, error) { | ||
| q := ` | ||
| WITH candidates AS ( |
There was a problem hiding this comment.
P3: The four new as-of methods duplicate the same reconstruction and filtering SQL, so future fixes can make counts disagree with page results. Extract the shared as-of CTE and filter expression.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/treatment_plan.go, line 607:
<comment>The four new as-of methods duplicate the same reconstruction and filtering SQL, so future fixes can make counts disagree with page results. Extract the shared as-of CTE and filter expression.</comment>
<file context>
@@ -517,6 +595,321 @@ WHERE %s
+ filter *TreatmentPlanFilter,
+) (int, error) {
+ q := `
+WITH candidates AS (
+ SELECT DISTINCT
+ treatment_plan_id
</file context>
| FROM | ||
| locked | ||
| INNER JOIN | ||
| risks r ON r.id = locked.risk_id |
There was a problem hiding this comment.
P3: This new treatment-plan loader crosses the coredata entity boundary by joining risks. Fetch the risk data separately and keep this treatment-plan query limited to treatment_plans.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/coredata/treatment_plan.go, line 242:
<comment>This new treatment-plan loader crosses the coredata entity boundary by joining `risks`. Fetch the risk data separately and keep this treatment-plan query limited to `treatment_plans`.</comment>
<file context>
@@ -188,6 +188,84 @@ LIMIT 1;
+FROM
+ locked
+INNER JOIN
+ risks r ON r.id = locked.risk_id
+`
+
</file context>
8766154 to
bc899c0
Compare
There was a problem hiding this comment.
7 existing issues remain and 2 new issues found across 52 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/probo/measure_service.go">
<violation number="1" location="pkg/probo/measure_service.go:617">
P2: When an update changes only `Description`, this guard skips `insertMeasureEvent`. As-of reads then report the earlier `UpdatedAt` and miss the edit; detect description changes too and persist the field in the measure event snapshot.</violation>
</file>
<file name="e2e/console/risk_analysis_test.go">
<violation number="1" location="e2e/console/risk_analysis_test.go:1105">
P2: The as-of assertions compare server-recorded event timestamps against a client wall clock that is captured with nanosecond precision immediately after the preceding mutations, leaving no skew/margin. If the server clock runs slightly ahead of the test client (more than the last mutation's round-trip latency), the required events clock in after `asOf` and the assertions (TotalCount==1, state IMPLEMENTED, old category) fail intermittently. Consider asserting against event-derived timestamps or adding a small margin so the as-of boundary has slack.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 8 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| return fmt.Errorf("cannot update measure: %w", err) | ||
| } | ||
|
|
||
| if measure.Name != previousName || measure.State != previousState || measure.Category != previousCategory { |
There was a problem hiding this comment.
P2: When an update changes only Description, this guard skips insertMeasureEvent. As-of reads then report the earlier UpdatedAt and miss the edit; detect description changes too and persist the field in the measure event snapshot.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/probo/measure_service.go, line 617:
<comment>When an update changes only `Description`, this guard skips `insertMeasureEvent`. As-of reads then report the earlier `UpdatedAt` and miss the edit; detect description changes too and persist the field in the measure event snapshot.</comment>
<file context>
@@ -593,6 +614,19 @@ func (s MeasureService) Update(
return fmt.Errorf("cannot update measure: %w", err)
}
+ if measure.Name != previousName || measure.State != previousState || measure.Category != previousCategory {
+ if err := insertMeasureEvent(
+ ctx,
</file context>
| assert.Equal(t, 0, livePlan.Progress.Total) | ||
| assert.Equal(t, 1, asOfMatrixCellCount(live.Node.MatrixCells, "NET", 4, 4)) | ||
|
|
||
| asOf := time.Now().UTC().Format(time.RFC3339Nano) |
There was a problem hiding this comment.
P2: The as-of assertions compare server-recorded event timestamps against a client wall clock that is captured with nanosecond precision immediately after the preceding mutations, leaving no skew/margin. If the server clock runs slightly ahead of the test client (more than the last mutation's round-trip latency), the required events clock in after asOf and the assertions (TotalCount==1, state IMPLEMENTED, old category) fail intermittently. Consider asserting against event-derived timestamps or adding a small margin so the as-of boundary has slack.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At e2e/console/risk_analysis_test.go, line 1105:
<comment>The as-of assertions compare server-recorded event timestamps against a client wall clock that is captured with nanosecond precision immediately after the preceding mutations, leaving no skew/margin. If the server clock runs slightly ahead of the test client (more than the last mutation's round-trip latency), the required events clock in after `asOf` and the assertions (TotalCount==1, state IMPLEMENTED, old category) fail intermittently. Consider asserting against event-derived timestamps or adding a small margin so the as-of boundary has slack.</comment>
<file context>
@@ -1065,3 +1066,466 @@ func TestRiskAnalysisNode_WithBoundary(t *testing.T) {
+ assert.Equal(t, 0, livePlan.Progress.Total)
+ assert.Equal(t, 1, asOfMatrixCellCount(live.Node.MatrixCells, "NET", 4, 4))
+
+ asOf := time.Now().UTC().Format(time.RFC3339Nano)
+ asOfResult := queryPlans(&asOf, 1)
+ require.Equal(t, 1, asOfResult.Node.TreatmentPlans.TotalCount)
</file context>
bc899c0 to
b14b3aa
Compare
There was a problem hiding this comment.
7 existing issues remain and 3 new issues found across 52 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/server/api/console/v1/treatment_plan_resolvers.go">
<violation number="1" location="pkg/server/api/console/v1/treatment_plan_resolvers.go:400">
P2: When a measure linked at the selected date was deleted later, this as-of resolver returns it but nested `Measure.treatmentPlans` authorizes against the deleted live measure ID and fails. Authorize historical measures through their surviving risk-analysis/organization scope before resolving their historical treatment plans.</violation>
</file>
<file name="apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_components/TreatmentPlanListItem.tsx">
<violation number="1" location="apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_components/TreatmentPlanListItem.tsx:185">
P2: When a risk category changes after its treatment plan is created, the live list shows the stale `treatmentPlan.category` instead of the current risk category. Use the live `risk.category` when no `asOf` date is selected, and reserve `treatmentPlan.category` for historical rows.</violation>
</file>
<file name="apps/console/src/pages/organizations/risks/risk-analyses/_components/RiskAnalysisPlansSection.tsx">
<violation number="1" location="apps/console/src/pages/organizations/risks/risk-analyses/_components/RiskAnalysisPlansSection.tsx:407">
P2: When viewing a historical date, expanding a plan still exposes `TreatmentPlanMeasureList` actions because `readOnly` stops only plan-level actions. Propagate the read-only state to the measure list and suppress its create, update, and delete controls.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 7 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| return nil, gqlutils.Internal(ctx) | ||
| } | ||
|
|
||
| connection := types.NewMeasureConnectionAsOf(page, r, obj.ID, measureFilter, *asOf, total) |
There was a problem hiding this comment.
P2: When a measure linked at the selected date was deleted later, this as-of resolver returns it but nested Measure.treatmentPlans authorizes against the deleted live measure ID and fails. Authorize historical measures through their surviving risk-analysis/organization scope before resolving their historical treatment plans.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/server/api/console/v1/treatment_plan_resolvers.go, line 400:
<comment>When a measure linked at the selected date was deleted later, this as-of resolver returns it but nested `Measure.treatmentPlans` authorizes against the deleted live measure ID and fails. Authorize historical measures through their surviving risk-analysis/organization scope before resolving their historical treatment plans.</comment>
<file context>
@@ -311,6 +377,31 @@ func (r *treatmentPlanResolver) Measures(ctx context.Context, obj *types.Treatme
+ return nil, gqlutils.Internal(ctx)
+ }
+
+ connection := types.NewMeasureConnectionAsOf(page, r, obj.ID, measureFilter, *asOf, total)
+
+ return connection, nil
</file context>
| </div> | ||
| </Td> | ||
| <Td className="w-px whitespace-nowrap pr-6">{treatmentPlan.risk.category}</Td> | ||
| <Td className="w-px whitespace-nowrap pr-6">{treatmentPlan.category}</Td> |
There was a problem hiding this comment.
P2: When a risk category changes after its treatment plan is created, the live list shows the stale treatmentPlan.category instead of the current risk category. Use the live risk.category when no asOf date is selected, and reserve treatmentPlan.category for historical rows.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/risks/risk-analyses/treatment-plans/_components/TreatmentPlanListItem.tsx, line 185:
<comment>When a risk category changes after its treatment plan is created, the live list shows the stale `treatmentPlan.category` instead of the current risk category. Use the live `risk.category` when no `asOf` date is selected, and reserve `treatmentPlan.category` for historical rows.</comment>
<file context>
@@ -194,7 +182,7 @@ export function TreatmentPlanListItem({
</div>
</Td>
- <Td className="w-px whitespace-nowrap pr-6">{treatmentPlan.risk.category}</Td>
+ <Td className="w-px whitespace-nowrap pr-6">{treatmentPlan.category}</Td>
<Td className="w-px whitespace-nowrap pr-6">
{t(`formRiskDialog.treatments.${treatmentPlan.treatment.toLowerCase()}`)}
</file context>
| treatmentPlanKey={row.plan} | ||
| connectionId={connectionId} | ||
| matrixSize={matrixSize} | ||
| readOnly={hasAsOf} |
There was a problem hiding this comment.
P2: When viewing a historical date, expanding a plan still exposes TreatmentPlanMeasureList actions because readOnly stops only plan-level actions. Propagate the read-only state to the measure list and suppress its create, update, and delete controls.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/console/src/pages/organizations/risks/risk-analyses/_components/RiskAnalysisPlansSection.tsx, line 407:
<comment>When viewing a historical date, expanding a plan still exposes `TreatmentPlanMeasureList` actions because `readOnly` stops only plan-level actions. Propagate the read-only state to the measure list and suppress its create, update, and delete controls.</comment>
<file context>
@@ -239,109 +335,174 @@ function RiskAnalysisPlansTable({
+ treatmentPlanKey={row.plan}
+ connectionId={connectionId}
+ matrixSize={matrixSize}
+ readOnly={hasAsOf}
+ onChanged={reload}
+ />
</file context>
Pick a past date and see the heatmap, table, and measures as they were. Each event stores the full plan so as-of reads the latest row. Owners and risks stay required; deleting either while an event still references them is restricted. Today stays on live data. Signed-off-by: Sacha Al Himdani <sacha@probo.com>
b14b3aa to
e537465
Compare
There was a problem hiding this comment.
5 existing issues remain and 1 new issue found across 52 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/riskmanagement/treatment_plan.go">
<violation number="1" location="pkg/riskmanagement/treatment_plan.go:284">
P2: When a risk category changes after this plan is created, as-of treatment-plan reads keep the old category while live reads show the new one. Emit a treatment-plan snapshot when the risk category changes, or otherwise add category history so the reconstructed plan reflects that edit.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 5 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| InherentImpact: req.InherentImpact, | ||
| ResidualLikelihood: residualLikelihood, | ||
| ResidualImpact: residualImpact, | ||
| Category: risk.Category, |
There was a problem hiding this comment.
P2: When a risk category changes after this plan is created, as-of treatment-plan reads keep the old category while live reads show the new one. Emit a treatment-plan snapshot when the risk category changes, or otherwise add category history so the reconstructed plan reflects that edit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/riskmanagement/treatment_plan.go, line 284:
<comment>When a risk category changes after this plan is created, as-of treatment-plan reads keep the old category while live reads show the new one. Emit a treatment-plan snapshot when the risk category changes, or otherwise add category history so the reconstructed plan reflects that edit.</comment>
<file context>
@@ -280,6 +281,7 @@ func (s *Service) CreateTreatmentPlan(
InherentImpact: req.InherentImpact,
ResidualLikelihood: residualLikelihood,
ResidualImpact: residualImpact,
+ Category: risk.Category,
CreatedAt: now,
UpdatedAt: now,
</file context>
Pick a past date and see the heatmap, table,
and measures as they were. Each event stores
the full plan so as-of reads the latest row.
Owners and risks stay required; deleting
either while an event still references them
is restricted. Today stays on live data.
Summary by cubic
Risk analyses can now reconstruct their heatmap, treatment plans, and linked measure states at a chosen instant instead of always reading current tables. Live views stay unchanged; historical results are read-only, and owners and risks cannot be deleted while snapshots still reference them.
Tests
+1162-3Covers matrix reconstruction, plan and measure history, forked plans, and access control for historical requests.
Coredata
+1816-32Adds event snapshots, a migration, and historical filters and loaders for treatment plans and measures.
GraphQL API
+305-13Exposes
asOfon matrix cells, treatment plans, and measures, including historical plan category and progress.MCP
+106-2Adds
as_ofto risk-analysis and treatment-plan tools, returns historical categories and linked measures, and rejects invalid scopes.prb (CLI)
+60-19Adds
--as-ofto risk-analysis and treatment-plan commands and includes plan category in output.Service
+910-4Records plan and measure changes, reconstructs historical matrix data, and preserves plan categories and snapshots when forking.
App: console
+671-236Adds localized As of controls, historical progress and measure views, and hides editing actions for read-only plans.
Package:
n8n-node+50-5Adds As Of inputs for risk-analysis and scoped treatment-plan retrieval and returns plan category.
Written for commit e537465. Summary will update on new commits.